Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OCM-9885 | feat: add validation for node pool custom disk size #70

Merged

Conversation

willkutler
Copy link
Contributor

for https://issues.redhat.com/browse/OCM-1027

Adds separate validation for custom root disk size for node pools. We want to keep the validation separate from classic machine pools since this validation does not require version and the min and max disk size may eventually be different in HCP from classic

Signed-off-by: wkutler <willkutler@gmail.com>
@hunterkepley
Copy link
Contributor

/lgtm

@@ -17,6 +17,9 @@ const (
// machinePoolRootVolumeSizeMaxAsOf414 is the maximum size of the root volume as of 4.14
// 16 TiB - limit as of 4.14
machinePoolRootVolumeSizeMaxAsOf414 = 16384
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that the same value?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, is there a difference from topology or is it only an OCP thing? Should we separate just to be safer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we just want to keep them separate in case we want to change the requirements to be different for HCP. the minimum specifically may be lower than classic but for now I'm just using the same values as classic so we can get the API changes in sooner

@davidleerh
Copy link
Contributor

/lgtm

@davidleerh davidleerh merged commit 64c0dff into openshift-online:main Aug 1, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants